Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WinVersion: add machine architecture property #14439

Closed
josephsl opened this issue Dec 12, 2022 · 1 comment · Fixed by #14464
Closed

WinVersion: add machine architecture property #14439

josephsl opened this issue Dec 12, 2022 · 1 comment · Fixed by #14464
Labels
audience/nvda-dev PR or issue is relevant to NVDA / Add-on developers p5 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority triaged Has been triaged, issue is waiting for implementation.
Milestone

Comments

@josephsl
Copy link
Collaborator

Hi,

Stemming from #14397:

Background:

Over the years, Windows supported various processor architectures from various vendors, including Intel x86 (32-bit), AMD64, Itanium (now discontinued), and ARM family (32-bit and 64-bit). While the base machine architecture for NVDA is x86, it provides support for x64 (Intel and AMD) and ARM64 through remote helper libraries.

While #14397 highlighted the need to improve support for ARM64 machines, it brought up an unspoken questino: can NVDA effectively detect different machine architectures? Moreover, thinking about this issue led me to believe that it would be helpful to let NVDA report machine architecture as part of Windows version information, similar to what Resource Monitor add-on provides.

Is your feature request related to a problem? Please describe.

At the moment NVDA does recognize workstation versus server Windows releases, but not machine architecture, which could have helped debug #14397 and similar issues.

Describe the solution you'd like

Add machine architecture (WinVersion.architecture) property to WinVersion class and report it at startup. This allows developers to figure out if the issue is specific to an architecture such as AMD64 or AM64, as well as detect test machines easily once ARM machines are used more on the cloud (currently virtual machines on the cloud are powered by x64 processors).

In addition, the following changes can be made:

  1. App modules: 64-bit process dtection becoes easier thanks to checking for Windows machine architecture check.
  2. Update check: made simpler by fetching WinVersion property.

Describe alternatives you've considered

Leave code as is and rely on environment variables to detect machine architecture.

Additional context

For an example, see josephsl/resourcemonitor, specifically Windows version script (NVDA+Shift+number row 6).

Proposed solution:

Bring machine architecture detection code from Resource Monitor add-on to NVDA Core, likely to target 2023.1 or 2023.2. I'd be happy to mentor someone to bring this feature request to life.

Thanks.

@seanbudd seanbudd added p5 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority audience/nvda-dev PR or issue is relevant to NVDA / Add-on developers triaged Has been triaged, issue is waiting for implementation. labels Dec 13, 2022
@josephsl
Copy link
Collaborator Author

Hi,

An enhanced solution in four lines:

import platform

Then define architecture attribute (constructor parameter and the constructor body itself, two lines), then in getWinVer function:

architecture = platform.machine()

The platform module became part of NVDA in 2019.3 with migration to Python 3, likely coming in as part of updated Py2exe dependency or via another one.

Thanks.

josephsl added a commit to josephsl/nvda that referenced this issue Dec 20, 2022
…cess#14439.

Define 'processorArchitecture' property in WinVersion class to store machine arcthiecture for the Windows installatoin (x86/32-bit, AMD64, ARM64). This is useful in detecting 64-bit systems and different architectures for use for log output, 64-bit versus 32-bit differences, and testing and debugging.
josephsl added a commit to josephsl/nvda that referenced this issue Dec 20, 2022
…#14439.

Print processor architecture for the Windows installation as part of Windows version output.
josephsl added a commit to josephsl/nvda that referenced this issue Dec 20, 2022
…ine() function. Re nvaccess#14439.

Python's platform module provides platform.machine() function, returing the processor architecture for the system. If Windows is detected, it internally retrieves PROCESSOR_ARCHITEW6432 and PROCESSOR_ARCHITECTURE environment variabls, in that order so it can detect WoW64 (such as x86 program running on an x64 processor). Because winVersion.getWinVer() function caches the current Windows installatoin, processor architecture string is always available and cached as part of the resulting WinVersion object. The practical implication is that NVDA no longer needs to consult environment variables every time it wants to detect processor architecture as the result is already known at program startup.
josephsl added a commit to josephsl/nvda that referenced this issue Dec 20, 2022
…r. Re nvaccess#14439.

Add winVersion.getWinVer().processorArchitecture test to see if what it says matches processor architecture (and WoW64 version) environment variable. Python's platform.machine() function is not used because that function consults environment variables itself.
seanbudd added a commit that referenced this issue Dec 21, 2022
Closes #14439
Follow-up to #14397
Follow-up to #14403

Summary of the issue:
winVersion.WinVersion class should define processor architecture property to record machine architecture for the current Windows installation.

Description of user facing changes
At NVDA startup, processor architecture will be logged as part of Windows information.

Description of development approach
Add "processorArchitecture" property to winVersion.WinVersion class to record machine architecture (currently x86 (32-bit) AMD64 (x64), ARM64). From getWinVer() function, this value will be fetched via Python's platform.machine() function, which itself looks up processor architecture from environment variables. This means NVDA can read the new property instead of consulting environment variables every time it wishes to check machine architecture. Also, added a unit test to see if the processor architecture recorded is indeed the one Windows says.

Commits:

* winVersion.WinVersion: define processor architecture property. Re #14439.

Define 'processorArchitecture' property in WinVersion class to store machine arcthiecture for the Windows installation (x86/32-bit, AMD64, ARM64). This is useful in detecting 64-bit systems and different architectures for use for log output, 64-bit versus 32-bit differences, and testing and debugging.

* WinVersion.repr: print processor architecture if defined. Re #14439.

Print processor architecture for the Windows installation as part of Windows version output.

* winVersion.getWinVer: obtain processor architecture via platform.machine() function. Re #14439.

Python's platform module provides platform.machine() function, returning the processor architecture for the system. If Windows is detected, it internally retrieves PROCESSOR_ARCHITEW6432 and PROCESSOR_ARCHITECTURE environment variabls, in that order so it can detect WoW64 (such as x86 program running on an x64 processor). Because winVersion.getWinVer() function caches the current Windows installation, processor architecture string is always available and cached as part of the resulting WinVersion object. The practical implication is that NVDA no longer needs to consult environment variables every time it wants to detect processor architecture as the result is already known at program startup.

* Unit tets: add processor architecture test and update copyright header. Re #14439.

Add winVersion.getWinVer().processorArchitecture test to see if what it says matches processor architecture (and WoW64 version) environment variable. Python's platform.machine() function is not used because that function consults environment variables itself.

* update changes

Co-authored-by: Sean Budd <sean@nvaccess.org>
@nvaccessAuto nvaccessAuto added this to the 2023.1 milestone Dec 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
audience/nvda-dev PR or issue is relevant to NVDA / Add-on developers p5 https://github.com/nvaccess/nvda/blob/master/projectDocs/issues/triage.md#priority triaged Has been triaged, issue is waiting for implementation.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants